Skip to content

fix(nextly): stop localized writes overwriting default-locale content - #382

Merged
mobeenabdullah merged 20 commits into
mainfrom
fix/localized-write-without-companion
Jul 30, 2026
Merged

fix(nextly): stop localized writes overwriting default-locale content#382
mobeenabdullah merged 20 commits into
mainfrom
fix/localized-write-without-companion

Conversation

@mobeenabdullah

@mobeenabdullah mobeenabdullah commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Closes the P0 found on published 0.0.2-alpha.43 during the alpha demo walk: saving a translation could silently overwrite the original language.

Scope note. This PR originally also seeded the companion table from existing content. That half has been split out into its own task after five review rounds established it needs a piece of state the system does not currently record (see "What this deliberately does not do"). What remains is the safety half, which has been stable since the first review round.

The defect

ensureCompanionTable is the intended db:sync/dev-boot counterpart to migration-owned companion creation, but it was only ever called at boot.

nextly db:sync runs in a separate CLI process. It flips dynamic_collections.<slug>.localized to 1 and returns. So in the window between that and the next boot:

  • the already-running dev server reloads the registry, sees localized: 1, registers dc_posts_locales, and renders the complete localization UI;
  • the physical companion table does not exist;
  • a non-default-language update falls through to the main table, overwriting the default language's values and regenerating the slug from the translation, reporting success.

Measured on alpha.43:

after db:sync, before restart:
  registry localized = 1
  locale tables in DB = []              <-- companion missing
  save es title -> dc_posts.title = "Cómo crear un blog con Nextly"
                   dc_posts.slug  = "cmo-crear-un-blog-con-nextly"
  (English title now only in a nextly_versions snapshot)

Every surface reported success: the CLI said "Schema applied — no changes", the admin rendered a correct localization UI, and the save succeeded.

The fix

1. Refuse the write. A write in a non-default language is rejected with NextlyError.conflict and an actionable message rather than falling through to the main row. Covers collections, singles and embedded field groups — singles failed differently (the split dropped the translation silently instead of overwriting), and a localized field group under a non-localized parent bypasses both of those guards, so it is guarded at the single point where its companion is written.

The default language still writes to the main table until the companion exists, which is the documented pre-migration fallback.

2. Create the companion in-process. db:sync and the dev config-reload path (next dev HMR) now provision the _locales table for every localized collection, single and field group. It must run after all three syncs, because a companion carries a foreign key to its main table.

3. Stop swallowing failures. ensureCompanionTable had a blanket catch {}, so a persistent failure was indistinguishable from first-boot ordering. It takes an optional reporter; db:sync logs a warning naming the entity and the consequence.

Also included: a leftover P2 from #375resetWebhookRecordingPolicy zeroed the refresh generation, so a refresh still in flight could match a later boot's counter and republish a stale decision. The counter now only increases.

What this deliberately does not do

It does not seed the companion from existing content, so enabling localization on an entity that already has content leaves that content invisible until the follow-up lands — the values stay intact on the main table, but reads resolve through the empty companion.

That is tracked as its own task. The short version of why it is not here: nothing records whether an entity's localization transition has completed, so every way of inferring it from the physical shape has a counter-example — a changed defaultLocale, a partially translated companion, MySQL's auto-committed DDL after a failed backfill, or a field the user deliberately cleared. Five review rounds each found the previous inference's blind spot, and by the end the requirements were mutually exclusive. It needs recorded transition state, which is a design change rather than another guess.

This PR still removes the destructive behaviour: content can no longer be overwritten or silently dropped. It can be hidden, which the follow-up closes.

Verification

Every regression test was run with its fix disabled and observed to fail.

Test Without the fix
Two-boot SQLite transition (content on main, es update) expected true to be false — the write silently succeeded and overwrote
Same scenario on sqlite / postgresql / mysql expected 500 to be 409 — an opaque driver error, not our refusal
Localized single, default-language write with no companion value silently dropped; the write reported success having saved nothing
db:sync creates a localized collection's companion expected false to be true
db:sync creates a localized single's companion expected false to be true
Custom dbName resolves to the canonical table expected false to be true
Non-localized collection gets no companion (negative control) passes either way, as intended
  • 3-dialect integration: SQLite, Postgres 17, MySQL, on freshly recreated databases.
  • check-types 0 errors, lint clean.

Post-review update (round 8)

Six further findings, all in the guard half. They were one question in six places — can
the main table actually persist these values right now?
— so they share a fix: a new
mainTableHasColumn in domains/i18n/runtime/companion-io.ts, built on
introspectLiveSnapshot (the same introspection the schema pipeline and migrate:resolve
use) rather than a SELECT ... LIMIT 0 probe. That choice settles a P2 for free: a probe
cannot tell a missing column from an unreachable database, so a transient failure used to
surface as a misleading "translations are not ready" refusal.

Applied for collections, singles and field groups, and resolved before the caller opens
its transaction. Field groups additionally get assertLocalizedFieldGroupsWritable, wired
at all three transactional callers of saveComponentDataInTransaction, and their
in-transaction splits probe on the transaction's own connection — signalled by an explicit
optional txAdapter rather than by comparing adapter identity, so a future caller cannot
silently reintroduce the pooling hazard.

Two review claims that did not survive measurement

Both fixes are kept — they are correct and strictly safer. But the stated reasons were
wrong, and the code comments have been rewritten to claim only what is verifiable.

1. The 409 is not lost when the refusal is raised inside a transaction. The finding
said a NextlyError thrown in a transaction callback is reclassified into an opaque
database error. Measured against the pre-fix code on Postgres and MySQL, the refusal still
came back as a clean 409: buildSingleErrorResult reads error.statusCode under
NextlyError.is(error), and the error still satisfies it. classifyError does rewrap
non-DatabaseError values, but it does not cost this refusal its status code. The
pre-transaction move is therefore defensive — its remaining benefit is small-pool
starvation, whose symptom is a hang rather than a wrong value. No test covers it, because
reproducing it needs a pool-size-1 adapter and the assertion would be a timeout.

2. The main-column path does not silently discard values. The finding predicted "a
write containing shared fields can succeed while silently discarding the localized values".
A test was built specifically to reproduce that — a collection localized from creation,
companion dropped, updated in the default locale with a shared author beside the
translatable title, so author alone would still form a valid UPDATE. Measured pre-fix:
expected 500 to be 409. The statement still carries a key the table has no column for, so
it fails at the driver rather than committing a partial row.

The first version of that test was invalid, which is worth recording. It declared the
shared field as plain text({ name: "author" }) — but in a localized collection a text
field localizes by default (defaultLocalizedForType), so author sat on the companion
too and the main table had no shared column at all, guaranteeing the 500 for the wrong
reason. It surfaced only because the follow-up SELECT author FROM dc_i18nwin_mixed failed
with no such column: author. The fixture now sets localized: false explicitly; re-measured
with a real shared column present, the result is unchanged.

So this check converts an opaque 500 into an actionable 409. Worth having, but error
quality rather than data-loss prevention. The PR's headline claim is unaffected: it rests on
the non-default-locale refusal, where a translation genuinely did overwrite the original
language while reporting success. The changeset line claiming default-language writes were
"unchanged" was false once this check existed, and now names the exception.

A defect this introduced, caught and fixed before merge

The first version of assertLocalizedFieldGroupsWritable scoped itself to a dynamic zone's
permitted types rather than the types present in the payload, and checked component
before components where saveComponentData checks the reverse. Two consequences:

  • it probed types the write never touches, adding a round trip per permitted type; and
  • more seriously, it refused valid saves — a zone permitting a localized type whose
    companion happened to be missing would 409 a write whose payload contained only a
    different, healthy type.

It now mirrors the real dispatch: components first, slugs derived from each instance's
_componentType, deduplicated. Covered by a test that saves a healthy block type while
another permitted type's companion is missing; against the previous implementation it fails
with expected true to be false.

Added tests

Test Without the fix
Collection localized from creation, default-locale update, companion dropped expected 500 to be 409
Same, with a shared field beside the translation (partial-write shape) expected 500 to be 409, on sqlite, Postgres and MySQL
Localized single from creation, default-language write, all dialects passes pre-fix — kept as a cross-dialect behavioural lock, not a discriminating test

Re-verified on the rebased tree: 3-dialect on freshly recreated databases, check-types and
lint clean.

Summary by CodeRabbit

  • New Features
    • _locales companion tables are automatically provisioned during db:sync, dev watch re-sync, and non-production config reloads (when auto-sync is enabled).
    • Companion _locales columns are reconciled via runtime schema introspection to add only missing localized columns.
  • Bug Fixes
    • Localized writes are blocked with clear 409 Translations are not ready conflicts when companion tables/columns aren’t ready, including stricter default-locale fallback handling.
    • Localized field-group readiness is pre-validated before transactions to ensure consistent routing.
    • Webhook refresh policy generation is now monotonic.
  • Tests
    • Added/updated integration tests for localized companion provisioning and missing-companion transition behavior across dialects.

@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR provisions localized companion tables during CLI sync, watch-mode resync, and configuration reloads. It adds physical readiness checks that reject unsafe localized writes, expands integration coverage, and makes webhook recording generations monotonic.

Changes

Localized companion lifecycle

Layer / File(s) Summary
Companion introspection and error reporting
packages/nextly/src/domains/i18n/runtime/companion-io.ts
Adds live schema introspection, companion-column reconciliation, and provisioning error callbacks.
Sync and reload provisioning
packages/nextly/src/cli/commands/*, packages/nextly/src/init/reload-config.ts
Ensures localized companions after CLI, watcher, and reload synchronization, while deferring unsafe schema transitions.
Localized mutation readiness
packages/nextly/src/domains/{collections,singles,field-groups}/services/*
Checks companion availability before writes, supports valid default-locale fallback, and rejects unavailable translation writes.
Missing-companion integration coverage
packages/nextly/src/cli/commands/__tests__/*, packages/nextly/src/domains/i18n/**, packages/nextly/src/domains/singles/**
Verifies provisioning, refusal, fallback persistence, no partial writes, dialect behavior, and field-group scoping.

Webhook recording generation

Layer / File(s) Summary
Monotonic recording reset generation
packages/nextly/src/domains/webhooks/recording-policy.ts, packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts
Resets increment the generation, and tests verify stale refresh decisions cannot overwrite newer decisions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigReload
  participant SchemaSync
  participant CompanionProvisioning
  participant Database
  ConfigReload->>SchemaSync: apply configuration schema changes
  SchemaSync-->>ConfigReload: return sync result
  ConfigReload->>CompanionProvisioning: provision localized companions
  CompanionProvisioning->>Database: create and reconcile _locales tables
  Database-->>CompanionProvisioning: report schema result
Loading

Possibly related PRs

Suggested reviewers: faisal-rx

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and omits the Summary, Type of change, Related issues, Changeset, and Checklist sections. Rewrite the PR description using the repository template and add the missing sections, especially Type of change, Related issues, Changeset, Test plan, and Checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 76.47% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main user-facing fix: preventing localized writes from overwriting default-locale content.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/localized-write-without-companion

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added scope: core nextly type: docs Documentation only labels Jul 28, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jul 28, 2026

Copy link
Copy Markdown

Open in StackBlitz

@nextlyhq/adapter-drizzle

npm i https://pkg.pr.new/@nextlyhq/adapter-drizzle@e33135f

@nextlyhq/adapter-mysql

npm i https://pkg.pr.new/@nextlyhq/adapter-mysql@e33135f

@nextlyhq/adapter-postgres

npm i https://pkg.pr.new/@nextlyhq/adapter-postgres@e33135f

@nextlyhq/adapter-sqlite

npm i https://pkg.pr.new/@nextlyhq/adapter-sqlite@e33135f

@nextlyhq/admin

npm i https://pkg.pr.new/@nextlyhq/admin@e33135f

@nextlyhq/admin-css

npm i https://pkg.pr.new/@nextlyhq/admin-css@e33135f

@nextlyhq/blocks-engine

npm i https://pkg.pr.new/@nextlyhq/blocks-engine@e33135f

create-nextly-app

npm i https://pkg.pr.new/create-nextly-app@e33135f

nextly

npm i https://pkg.pr.new/nextly@e33135f

@nextlyhq/plugin-form-builder

npm i https://pkg.pr.new/@nextlyhq/plugin-form-builder@e33135f

@nextlyhq/plugin-page-builder

npm i https://pkg.pr.new/@nextlyhq/plugin-page-builder@e33135f

@nextlyhq/plugin-sdk

npm i https://pkg.pr.new/@nextlyhq/plugin-sdk@e33135f

@nextlyhq/plugin-seo

npm i https://pkg.pr.new/@nextlyhq/plugin-seo@e33135f

@nextlyhq/storage-s3

npm i https://pkg.pr.new/@nextlyhq/storage-s3@e33135f

@nextlyhq/storage-uploadthing

npm i https://pkg.pr.new/@nextlyhq/storage-uploadthing@e33135f

@nextlyhq/storage-vercel-blob

npm i https://pkg.pr.new/@nextlyhq/storage-vercel-blob@e33135f

@nextlyhq/ui

npm i https://pkg.pr.new/@nextlyhq/ui@e33135f

commit: e33135f

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe76865f50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/cli/commands/dev-build.ts Outdated
Comment thread packages/nextly/src/cli/commands/dev-build.ts
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts (1)

157-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider covering a localized component too.

Components are the one kind whose dbName is honored verbatim (resolveComponentTableName), which is exactly the divergence the dbName test guards against for collections — but no case exercises the component branch of ensureLocalizedCompanions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts`
around lines 157 - 181, Extend the localized companion integration coverage to
include a localized component with a custom dbName, exercising the component
branch of ensureLocalizedCompanions. Use resolveComponentTableName semantics by
asserting the companion table is derived from the verbatim component dbName, and
verify the incorrectly prefixed alternative is absent.
packages/nextly/src/domains/i18n/runtime/companion-io.ts (1)

246-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Doc block still claims failures are swallowed.

The behavior now reports through the optional onError (Lines 269–273, 312–322); "a failure ... is swallowed so it retries on the next boot" only holds for callers that omit the reporter. Worth qualifying so the contract reads consistently.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextly/src/domains/i18n/runtime/companion-io.ts` around lines 246 -
252, Update the documentation for the companion creation flow near its
best-effort behavior to clarify that failures are reported through the optional
onError callback when provided, while callers omitting onError swallow the
failure and retry on the next boot. Keep the existing runtime behavior
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/nextly/src/di/register.ts`:
- Around line 476-479: Update the initializeSchemaRegistry call in the
surrounding registration flow to source localization.defaultLocale from
transformedConfig instead of config, preserving optional access. Align this
registry path with the existing ensureCompanionTable call sites that use
transformedConfig so plugin-transformed locale values are passed through.

In
`@packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts`:
- Around line 12-15: Remove the sentence referencing “published alpha.43” and
“task 006” from the header comment in
localized-write-without-companion.integration.test.ts. Keep the preceding
explanation of the non-default-locale failure mode unchanged.

---

Nitpick comments:
In
`@packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts`:
- Around line 157-181: Extend the localized companion integration coverage to
include a localized component with a custom dbName, exercising the component
branch of ensureLocalizedCompanions. Use resolveComponentTableName semantics by
asserting the companion table is derived from the verbatim component dbName, and
verify the incorrectly prefixed alternative is absent.

In `@packages/nextly/src/domains/i18n/runtime/companion-io.ts`:
- Around line 246-252: Update the documentation for the companion creation flow
near its best-effort behavior to clarify that failures are reported through the
optional onError callback when provided, while callers omitting onError swallow
the failure and retry on the next boot. Keep the existing runtime behavior
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 88abd0d2-28d6-45aa-af92-7a0235dae530

📥 Commits

Reviewing files that changed from the base of the PR and between 0498e02 and 38766c3.

⛔ Files ignored due to path filters (1)
  • .changeset/localized-write-companion-guard.md is excluded by !.changeset/**
📒 Files selected for processing (12)
  • packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts
  • packages/nextly/src/cli/commands/db-sync.ts
  • packages/nextly/src/cli/commands/dev-build.ts
  • packages/nextly/src/cli/commands/dev-watcher.ts
  • packages/nextly/src/di/register.ts
  • packages/nextly/src/domains/collections/services/collection-mutation-service.ts
  • packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts
  • packages/nextly/src/domains/i18n/migration/generate-up.ts
  • packages/nextly/src/domains/i18n/runtime/companion-io.ts
  • packages/nextly/src/domains/singles/services/single-mutation-service.ts
  • packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts
  • packages/nextly/src/domains/webhooks/recording-policy.ts

Comment thread packages/nextly/src/di/register.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38766c3381

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/cli/commands/db-sync.ts Outdated
Comment thread packages/nextly/src/cli/commands/dev-build.ts Outdated
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af53dbca1a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/collections/services/collection-mutation-service.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/singles/services/single-mutation-service.ts Outdated
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from af53dbc to 61b30b4 Compare July 29, 2026 03:07
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61b30b4bac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 30692efb2a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/cli/commands/dev-watcher.ts
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/cli/commands/db-sync.ts
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from 30692ef to 43eaf63 Compare July 29, 2026 04:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43eaf6381e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/init/reload-config.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba79cfee62

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/init/reload-config.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts
Comment thread packages/nextly/src/cli/commands/dev-build.ts
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from ba79cfe to 17c97d7 Compare July 29, 2026 07:29

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17c97d7eed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from 17c97d7 to 3a0c3f4 Compare July 29, 2026 08:26
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a0c3f4f1b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts
Comment thread packages/nextly/src/init/reload-config.ts Outdated
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from 3a0c3f4 to 1552920 Compare July 29, 2026 11:33
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

@codex please review this PR

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/nextly/src/init/reload-config.ts (1)

659-683: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add localization to the inline cast so the declared and assigned types agree.

Line 665 widens newConfig with localization?.defaultLocale, but the cast at Lines 674-683 still describes result.config without it. The property survives at runtime (the cast is structural), yet the source type says otherwise — so anyone tightening this cast, or reading it to decide whether a locale is available, will conclude ensureLocalizedCompanionsForReload can never see a defaultLocale.

🩹 Proposed fix
     newConfig = (
       result as {
         config?: {
           collections?: CollectionDef[];
           singles?: SingleDef[];
           fieldGroups?: ComponentDef[];
           webhookAuditEnabled?: boolean;
+          localization?: { defaultLocale?: string };
         };
       }
     ).config;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextly/src/init/reload-config.ts` around lines 659 - 683, Update the
inline type cast for result.config in the reload configuration flow to include
the optional localization object with its optional defaultLocale string,
matching the declared newConfig shape. Keep the existing fields and assignment
behavior unchanged.
🧹 Nitpick comments (4)
packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts (1)

73-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the mirror case: a non-default-locale single write must be refused.

This file only pins the default-language fallback. The PR also adds a missing-companion guard to single-mutation-service.ts, and nothing here exercises it — the collection equivalent (domains/i18n/__tests__/localized-write-without-companion.integration.test.ts) asserts success: false, 409 and the message, so the singles guard could regress silently. Asserting success on the existing updateSingle call would also make a failure point at the write rather than at the row read.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts`
around lines 73 - 100, Add a second integration test in the localized
single-without-companion suite covering an explicit non-default locale such as
"fr" after dropping single_swin_settings_locales. Assert updateSingle returns
success: false, status 409, and the missing-companion error message, rather than
relying on the subsequent row query; preserve the existing default-language
fallback test.
packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts (1)

184-204: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Probe is issued once per instance; consider caching it for the duration of the save.

upsertLocalizedComponent is called per component instance (saveRepeatableComponents*, saveMultiComponents*), so a repeatable localized field group with N instances runs N identical SELECT 1 FROM <companion> LIMIT 0 round trips against the same table. Memoizing the result per companion table name for the duration of one saveComponentData* call keeps the guard while collapsing it to one probe.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts`
around lines 184 - 204, The companion-table existence probe in
upsertLocalizedComponent currently repeats for every localized component
instance. Cache companionTableExists results by companion table name within each
saveComponentData* operation, pass or reuse that per-save memo across
saveRepeatableComponents* and saveMultiComponents* calls, and keep the existing
conflict behavior when the cached result is false.
packages/nextly/src/cli/commands/dev-build.ts (1)

789-879: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

One companion-provisioning walk implemented twice. Both functions carry the same production guard, the same three [entities, resolveTableName] groups (collections → resolveCollectionTableName, singles → resolveSingleTableName, field groups → resolveComponentTableName), the same Localizable/LocalizableEntity shape and the same ensureCompanionTable argument set including reconcileExisting: true. Any future change — a fourth entity kind, a different reconcile policy, a fix to dbName resolution — will land in one copy only.

  • packages/nextly/src/cli/commands/dev-build.ts#L789-L879: extract the shared walk (taking an ensureCompanionTable-compatible adapter and a warn callback) into a helper next to companion-io.ts, and reduce this function to the CLI logger wiring.
  • packages/nextly/src/init/reload-config.ts#L569-L639: call the same helper, keeping only the console.warn reporter for the HMR path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextly/src/cli/commands/dev-build.ts` around lines 789 - 879, The
companion-provisioning walk is duplicated across both callers. In
packages/nextly/src/cli/commands/dev-build.ts lines 789-879, extract the shared
production guard, entity groups, table resolution, and ensureCompanionTable
iteration into a helper next to companion-io.ts that accepts an
ensureCompanionTable-compatible adapter and warning callback, then retain only
CLI logger wiring in ensureLocalizedCompanions. In
packages/nextly/src/init/reload-config.ts lines 569-639, replace the duplicate
walk with the shared helper and preserve console.warn as the HMR reporter.
packages/nextly/src/domains/i18n/runtime/companion-io.ts (1)

330-358: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Hoist the alreadyExists && !reconcileExisting early return above the import + introspection.

The plain boot path (di/register.ts) never passes reconcileExisting, so for every already-provisioned localized collection/single/component it pays a dynamic import plus a full introspectLiveSnapshot (two pg_*/information_schema queries on PG, PRAGMA round trips on SQLite) and then returns without doing anything. Only the existence probe is needed to reach that decision.

♻️ Proposed reordering
   const companionTableName = `${args.tableName}_locales`;
   try {
     const alreadyExists = await companionTableExists(
       adapter,
       companionTableName
     );
+    // Reconciling an EXISTING companion is opt-in (see `reconcileExisting`). Boot
+    // only ever CREATEs, so bail before the import + introspection below rather
+    // than paying for them on every localized entity on every boot.
+    const reconcileExisting = args.reconcileExisting === true;
+    if (alreadyExists && !reconcileExisting) return;
     // Lazy import avoids a cycle (reconcile-companion → migration helpers).
     const { buildCompanionReconcileStatements } = await import(
       "../migration/reconcile-companion"
     );
@@
     const companionColumns = physical.get(companionTableName) ?? new Set();
-    // Reconciling an EXISTING companion is opt-in (see `reconcileExisting`). When it
-    // is off, treat the current columns as the desired ones so the reconcile emits
-    // nothing for a table that is already there — boot then only ever CREATEs.
-    const reconcileExisting = args.reconcileExisting === true;
-    if (alreadyExists && !reconcileExisting) return;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/nextly/src/domains/i18n/runtime/companion-io.ts` around lines 330 -
358, Move the alreadyExists && !reconcileExisting early return to immediately
after the existence check and before the dynamic import,
resolveLocalizedFieldNames work, and readPhysicalColumns introspection. Preserve
the existing behavior for new companions and for callers that explicitly enable
reconcileExisting, which must continue through reconciliation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts`:
- Around line 85-99: Update runSync to disconnect the existing adapter before
assigning the newly created adapter, ensuring repeated calls close the previous
SQLite handle while preserving cleanup of the current adapter in afterEach.

In
`@packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts`:
- Around line 408-410: Guard the describe.each calls for the localized-write
integration suites, including the blocks around the visible dialect suite and
its companion at the other referenced location, so empty results from
getConfiguredTestDialects() skip only dialect-dependent cases without producing
an empty suite. Preserve execution of the SQLite cases when neither
TEST_POSTGRES_URL nor TEST_MYSQL_URL is configured.

---

Outside diff comments:
In `@packages/nextly/src/init/reload-config.ts`:
- Around line 659-683: Update the inline type cast for result.config in the
reload configuration flow to include the optional localization object with its
optional defaultLocale string, matching the declared newConfig shape. Keep the
existing fields and assignment behavior unchanged.

---

Nitpick comments:
In `@packages/nextly/src/cli/commands/dev-build.ts`:
- Around line 789-879: The companion-provisioning walk is duplicated across both
callers. In packages/nextly/src/cli/commands/dev-build.ts lines 789-879, extract
the shared production guard, entity groups, table resolution, and
ensureCompanionTable iteration into a helper next to companion-io.ts that
accepts an ensureCompanionTable-compatible adapter and warning callback, then
retain only CLI logger wiring in ensureLocalizedCompanions. In
packages/nextly/src/init/reload-config.ts lines 569-639, replace the duplicate
walk with the shared helper and preserve console.warn as the HMR reporter.

In
`@packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts`:
- Around line 184-204: The companion-table existence probe in
upsertLocalizedComponent currently repeats for every localized component
instance. Cache companionTableExists results by companion table name within each
saveComponentData* operation, pass or reuse that per-save memo across
saveRepeatableComponents* and saveMultiComponents* calls, and keep the existing
conflict behavior when the cached result is false.

In `@packages/nextly/src/domains/i18n/runtime/companion-io.ts`:
- Around line 330-358: Move the alreadyExists && !reconcileExisting early return
to immediately after the existence check and before the dynamic import,
resolveLocalizedFieldNames work, and readPhysicalColumns introspection. Preserve
the existing behavior for new companions and for callers that explicitly enable
reconcileExisting, which must continue through reconciliation.

In
`@packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts`:
- Around line 73-100: Add a second integration test in the localized
single-without-companion suite covering an explicit non-default locale such as
"fr" after dropping single_swin_settings_locales. Assert updateSingle returns
success: false, status 409, and the missing-companion error message, rather than
relying on the subsequent row query; preserve the existing default-language
fallback test.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ccabb2f5-9dc3-4e5c-8d44-a9243db51235

📥 Commits

Reviewing files that changed from the base of the PR and between 38766c3 and 1552920.

⛔ Files ignored due to path filters (1)
  • .changeset/localized-write-companion-guard.md is excluded by !.changeset/**
📒 Files selected for processing (16)
  • packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts
  • packages/nextly/src/cli/commands/db-sync.ts
  • packages/nextly/src/cli/commands/dev-build.ts
  • packages/nextly/src/cli/commands/dev-watcher.ts
  • packages/nextly/src/di/register.ts
  • packages/nextly/src/domains/collections/services/collection-mutation-service.ts
  • packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts
  • packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts
  • packages/nextly/src/domains/i18n/migration/generate-up.ts
  • packages/nextly/src/domains/i18n/runtime/companion-io.ts
  • packages/nextly/src/domains/i18n/writes-create.integration.test.ts
  • packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts
  • packages/nextly/src/domains/singles/services/single-mutation-service.ts
  • packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts
  • packages/nextly/src/domains/webhooks/recording-policy.ts
  • packages/nextly/src/init/reload-config.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/nextly/src/domains/singles/services/single-mutation-service.ts
  • packages/nextly/src/cli/commands/db-sync.ts
  • packages/nextly/src/domains/webhooks/recording-policy.ts
  • packages/nextly/src/cli/commands/dev-watcher.ts
  • packages/nextly/src/domains/i18n/migration/generate-up.ts
  • packages/nextly/src/domains/collections/services/collection-mutation-service.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1552920b47

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/migration/generate-up.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/domains/i18n/runtime/companion-io.ts Outdated
Comment thread packages/nextly/src/init/reload-config.ts Outdated
Comment thread packages/nextly/src/cli/commands/dev-build.ts
@mobeenabdullah

Copy link
Copy Markdown
Collaborator Author

Rescoped: the companion seeding half has been split out into its own task, and this PR is now the write guard only (−694 lines). Please review the reduced surface.

@codex please review this PR

While a companion table is missing, a write in the default language is meant
to stay on the main table. That is only possible where the main table still
carries the translatable columns. An entity localized from creation keeps them
solely on the companion and its generated runtime table omits them, so the
fallback handed those keys to an ORM that does not declare them: they were
dropped, `updated_at` still moved, and the write reported success having saved
nothing.

Collections, singles and field groups now prove the column is physically
present before taking that fallback, and refuse with the same 409 when it is
not. The check goes through a shared `mainTableHasColumn` built on
`introspectLiveSnapshot` rather than a `SELECT ... LIMIT 0` probe: a probe
cannot tell a missing column from an unreachable database, so a transient
failure surfaced as a misleading "translations are not ready" refusal.

The checks resolve before the caller opens its transaction. Running them
inside would borrow a second connection while the transaction holds one, which
on a small pool means waiting for a connection that cannot be released until it
finishes; it also keeps a refusal exactly as raised rather than passing it
through the adapter's error classification on the way out. Field groups get
`assertLocalizedFieldGroupsWritable` for that, wired at all three transactional
callers, and their in-transaction splits probe on the transaction's own
connection, signalled by an explicit optional `txAdapter` rather than by
adapter identity.
Probing for a companion from inside a write transaction is not safe. The probe
asks about a relation that may not exist, and PostgreSQL marks the entire
transaction aborted the moment a statement errors — so although the probe
catches it and correctly reports "absent", every statement after it fails with
`current transaction is aborted`.

`assertLocalizedFieldGroupsWritable` now returns the existence it resolved, and
that map is threaded through `saveComponentDataInTransaction` into the three
in-transaction variants, which read it instead of asking again. It is a required
parameter rather than an optional one: inside a transaction there is no safe way
to work the answer out, so a caller that cannot supply it should not compile.
Per-write probes drop from `1 + 2K` to `1 + K` for K distinct localized field
group types.

Provisioning on the config-reload path now skips entities whose own schema change
was deferred. Creating a companion for a transition that was not applied is worse
than leaving it absent: the later Schema Builder apply finds the companion already
present, takes the plain reconcile branch instead of seeding, and the existing
default-locale values are lost. Tracked per entity rather than as a single flag,
so entities whose schema is in step are still provisioned on the same pass.

`db:sync` also reconciles an existing companion's columns rather than only
creating a missing table. Marking a further field localized on an already
localized entity adds the column to the main table while the companion keeps its
old shape, and the write then splits that value into a column that is not there.
Additive only — an unattended sync must not drop a column, because `db:sync`
persists registry metadata before its destructive prompt — and confined to
`db:sync`, leaving the reload path creation-only.
A dynamic zone defaults to `repeatable: false`, and then its payload is a single
object rather than an array. The pre-transaction check read that as "no
instances", so the slug never reached the presence map and the write went looking
for the companion table from inside the transaction — the exact failure the check
exists to prevent, and on PostgreSQL an aborted transaction.

The cause was two hand-mirrored normalizations drifting apart, so both now go
through one `resolveZoneInstances`: the check and the write can no longer disagree
about what a payload holds. The in-transaction default changes with it, from
"assume provisioned" to "assume absent" — the two are not equally safe to guess
wrong, because absent takes the fallback and fails loudly on the main table
whereas provisioned splits values into a table that may not exist.

The guard also refused more than it needed to. It rejected every non-default
locale write while the companion was missing, without first asking whether the
payload contained any translatable field at all. A shared-only edit touches
neither the absent table nor the default language, so it is now allowed through
at all three sites. Membership is decided by the canonical split, which accepts
either the camelCase field name or the snake_case companion column.

Companion reconciliation now tracks `_status` alongside the translatable columns,
since switching Draft/Published on after the companion was created otherwise
leaves per-locale status writes targeting a column that is not there. Reported as
wanted whenever it already exists, so the reconcile can add a missing `_status`
but never drop one from an unattended sync.

The config-reload path reconciles as well as creates. `ensureCompanionTable`
returns immediately for an existing table, so marking a further field localized
took the no-DDL path and left the companion a column short. Safe there despite
issuing DDL: the production guard at the top of that function has already
returned, so it runs only under `next dev`.
…tatus

Deciding per write whether a payload "touches the companion" turned out to need
more than the payload. Three ways it was wrong, each a way to lose content:

- Hooks run after the decision. A field-level `beforeChange` that adds a
  localized value to an otherwise shared-only update is persisted from the
  post-hook payload, so a non-default-locale write reached the main table and
  overwrote the default locale's values.
- `_status` is companion-owned per locale. A status-only PATCH therefore looked
  shared-only, and the status landed on the main row — unpublishing every locale
  at once on the collection path.
- The field-group preflight passes `{}` to reach its existence decision, so the
  bypass never applied there and embedded groups kept refusing anyway.

The refusal is unconditional again. What it costs is an editor being unable to
change a shared field while the companion is missing, which is a transient state
that `db:sync` or a reload resolves; what it buys back is three paths that could
destroy or hide content. The optimization needs the recorded transition state to
be done safely, so it waits for that rather than being inferred per write.

Companion reconciliation now passes `defaultLocale`, which is what lets the
builder emit its default-locale status backfill. ADD COLUMN seeds every existing
companion row at 'draft' including the default-locale row, but that row's status
IS the main row's and may already be 'published'. Without the backfill, enabling
Draft/Published on an entity that already has content made all of it read as
draft and drop out of published localized reads until each row was republished by
hand.

Also corrects documentation that still claimed the reload path seeds companions.
It creates and reconciles; existing content stays where it is, so a successful
reload is not evidence that default-locale data was carried across.
Adding `_status` to an existing companion is one statement and backfilling the
default-locale row from the main row is another, and physical shape cannot tell
the two apart afterwards. When the ADD lands and the backfill does not, every
later run sees the column present, concludes the companion is in step, and
returns — leaving previously published content reading as draft while reporting
success. MySQL commits DDL implicitly, so the pair cannot be made atomic there
either.

Deciding this correctly needs a record of whether the backfill has run, which is
state the system does not keep yet. Until it does, switching Draft/Published on
for an already-localized entity belongs to the migration path: that fails loudly
on a missing column, which is a better outcome than silently hiding published
rows.

The localized-column reconcile stays, because it has the opposite property. A
missing column is visible on every run, so a partial apply simply completes on
the next one. Retryability is the line between the two, and it is now stated
where the decision is made.
…isioning

The fallback check proved one column existed and inferred the rest. A partially
migrated main table — an older field keeping its legacy column while a newer
localized field never had one — passed on the first column and then failed at the
driver on a later one, which is the opaque error this check exists to replace.
`mainTableHasColumns` now requires all of them, at all three call sites.

Provisioning tolerates losing a race. `db:sync` and a dev boot or HMR reload
provision the same companions, and neither `CREATE TABLE` nor `ADD COLUMN` is
idempotent here, so whichever process arrived second failed the whole sync even
though the table it wanted now existed. Both paths re-read the schema after a
failure and treat the wanted shape being present as success, whoever produced it.
Decided by re-introspecting rather than by matching driver error text, so a real
failure that happens to mention the table is not swallowed.

Enabling Draft/Published on a companion that predates it no longer reports
success. Reconciling `_status` is unsafe here — the ADD and the default-locale
back-fill cannot be retried as a pair — but returning quietly was worse: the
caller persisted `status: true` and every later per-locale status read hit a
column that was not there. It now reports through the error channel, so the sync
exits non-zero and names `nextly migrate`. Only that direction; status switched
off with the column still present is harmless.

Also removes a duplicated test block that ran the non-repeatable dynamic-zone case
twice per dialect.
@mobeenabdullah
mobeenabdullah force-pushed the fix/localized-write-without-companion branch from 870bc8e to e33135f Compare July 30, 2026 05:41

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e33135fb1d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -942,6 +947,65 @@ export class CollectionMutationService extends BaseService {
// `migrate`, the dev auto-sync leaves localized columns on the MAIN table (Option B), so
// writes must go there — return null and let the localized values flow to main as today.
if (!(await this.companionTableExists(companion.companionTableName))) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Recheck the companion before committing fallback writes

When a default-locale create or update observes a missing companion here, a concurrent db:sync or HMR provisioning pass can create the table before the later main-table transaction commits. The write still follows the cached fallback decision and reports success after storing the value only on the main table, but subsequent reads now query the newly created companion and replace that field with null, making the edit disappear and vulnerable to overwrite. The same stale-presence pattern is used by Singles and field groups, so companion creation must be synchronized with fallback writes or the write must safely revalidate and reroute before committing.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, and I am not going to fix this one here — because I do not think it can be fixed at this layer, and the three findings together are what convinced me.

Taken as a set, the review has now closed off every option for deciding companion existence per write:

Where the question is asked What breaks
Inside the transaction Probing a missing relation aborts the whole transaction on PostgreSQL — your earlier P2
Before the transaction, cached and passed in This finding: a concurrent db:sync or HMR pass creates the table before the write commits, the write follows the stale decision, and later reads return null
Not asked at all The original P0 this PR exists to close: the write lands on the main table and overwrites the default language

Each fix moved the failure rather than removing it. That is a missing invariant, not three bugs — and the remaining alternative, serializing every localized write against provisioning, is a far worse trade than the window it would close.

What resolves it is making readiness state rather than an observation: resolved when the schema changes, cached, invalidated by the same events, read by provisioning and writes alike. That is the recorded-transition-state work this PR was split around, and I have added this race to it as the third independent proof, alongside the seeding counter-examples and the PostgreSQL abort.

Two things worth being precise about on severity, because they decide whether this blocks the merge:

The value is not destroyed. It is committed to the main table; the later read resolves through the newly created companion and overlays null. That is exactly the state the follow-up task already exists to fix — content intact on main, invisible through the companion — reached from a different direction rather than a new class of defect.

It is strictly better than the behaviour it replaces. On main today, that same window silently overwrites the default language's content with a translation and regenerates the slug from it, reporting success. This PR turns destruction into a narrow, dev-time visibility race. I would rather ship that improvement and close the race properly with recorded state than hold the destructive behaviour in place while the larger change is built.

So: acknowledged as a known limitation, recorded with the reproduction and the reasoning, and explicitly in scope for the follow-up rather than quietly dropped.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed real, and it arrived just after this merged — so recording it as a tracked follow-up rather than leaving it on a closed PR.

The race is exactly as described: the fallback decision is made before the transaction opens, a concurrent db:sync or HMR pass creates the companion inside that window, the write commits against the stale decision, and every later read resolves through the new empty companion and overlays null. Collections, singles and field groups all share the shape.

On the two suggested remedies, both of which I think are unsafe here — which is why this needs the follow-up rather than a patch.

Revalidate before committing means probing for the companion from inside the write transaction. That is the defect already tracked from this PR: on PostgreSQL a probe against a missing relation aborts the entire transaction, so the fallback write that follows dies with current transaction is aborted. Adding the revalidation would close this race by reintroducing that one.

Synchronizing provisioning with writes puts a cross-process lock on the write path to protect a state that exists only in a transient window — real cost, permanently, for a condition that should not persist.

So this is a fifth symptom of one missing primitive. Provisioning, the write guard, the read-back and now the fallback decision all ask "is this companion ready?" and each answers by querying at the moment it needs to know. The clean close is readiness resolved once and invalidated when provisioning changes it: then there is no cached-decision window to race against, because the decision and its invalidation share a source of truth.

Recorded on the transition-state task with an explicit acceptance criterion, since this is the one failure in the set that only appears under concurrency and so needs a test written for it deliberately.

Thank you for catching it after the merge rather than letting it sit.

@mobeenabdullah
mobeenabdullah merged commit b448e6d into main Jul 30, 2026
15 checks passed
mobeenabdullah added a commit that referenced this pull request Jul 30, 2026
…guage

Boot-time provisioning has no localization config, so it passed no source locale
and created the companion empty. Creation is a race, and reads resolve through
the companion once it exists — so whichever caller won decided whether existing
content stayed visible, and a boot that beat `db:sync` hid it permanently, with
the seeding path then finding the table already there and skipping forever.

A caller that cannot say which language the main table's content is in now
declines to create the companion at all when the main table still carries the
translatable columns, and reports why. The entity is left to the path that knows,
and #382's write guard keeps a non-default write from doing damage until then.

Closing this by refusing rather than by teaching every caller to supply a locale
keeps it closed for callers that do not exist yet.
mobeenabdullah added a commit that referenced this pull request Jul 31, 2026
…led (#419)

* feat(nextly): record which language an entity's content was written in

Enabling localization on an entity that already has content is a data
migration: the values sit on the main table in one language and have to be
copied into the companion labelled with it. That language cannot be recovered
afterwards. `defaultLocale` is the current default rather than the one in force
when the values were written, and an empty companion cannot distinguish a copy
that is owed from one that ran and produced nothing, so every physical shape a
reader can observe has a counter-example.

Store it instead, per entity in `nextly_meta`, with a status that separates a
copy still owed from one that finished. Written before the first statement,
because MySQL commits DDL implicitly and a post-hoc write would leave a
companion whose retry sees the table and skips the copy forever. A marker that
exists but cannot be read refuses rather than reading as absent, and the
recorded source locale cannot change once written.

Nothing reads it yet.

* feat(nextly): record the transition when db:sync creates a companion

`ensureCompanionTable` now reports whether THIS call created the table. That is
the only moment the current default locale is a safe answer to what language the
main table's content is in: before it the entity was not localized, and after it
nothing on disk can say what the default was when those values were written.

`db:sync` and the dev watcher record the transition on that signal alone.
Recording on every sync instead would attach today's default to older content,
and a confident wrong language is worse than no record — so the recorder is
refused a second write with a different locale as well, which makes the mistake
a failure rather than a silent relabel.

The entity kind travels with each provisioning group because it is part of the
record's key: a collection, a single and a field group may share a slug while
only one of them has transitioned.

Still nothing reads the record. The reload path and the four Builder dispatchers
do not write it yet.

* feat(nextly): record the transition on the dev reload path

The reload path provisions the same companions as `db:sync`, so it owes the same
record. Extracts the recorder into one module rather than repeating its
construction, since every path that can create a companion needs the same store
and the same locale, and sourcing those separately is how a transition ends up
labelled with a locale that was never in force.

* feat(nextly): allow seeding a companion without dropping the source columns

The enable plan copies the main table's values into the companion and then drops
the columns it copied from. That is right for an explicit transition — a Builder
toggle or a migration file — where relocating the data is the point.

Unattended provisioning cannot do the second half. Boot, `db:sync` and the dev
reload path are additive-only, and a dropped column is not something the next
boot can put back. Leaving the originals in place gives a companion that holds
the content and a main table carrying inert duplicates, which `nextly migrate`
can remove under supervision.

Drops by default, so both existing callers are unchanged.

* fix(nextly): keep existing content readable when localization is enabled

Enabling localization in `nextly.config.ts` on an entity that already had content
created an empty companion. Every localized read then resolved through it and
returned null, over values still sitting on the main table. The admin Schema
Builder never had this problem: its toggle has always copied those values into
the companion as it creates it. So the same product hid content or preserved it
depending on which way localization was turned on.

The code-first path now runs the same plan, seeding from the localized columns
that physically exist on the main table — physical shape decides, because a field
localized in the same change has no column to copy from and a field name is not a
column name.

The plan's column drops are suppressed here. This runs unattended from boot and
`db:sync`, which are additive-only, and a dropped column is not something the
next boot can put back. The copies left on main are inert once reads resolve
through the companion.

* test(nextly): cover the seed-without-drop plan on every dialect

The statements this emits differ per dialect in identifier quoting, and the path
that consumes them copies user content. Asserting only on SQLite would leave the
generation unverified for the two dialects where a mistake is hardest to notice.

* docs(release): changeset for the localization enable fix

* fix(nextly): seed the companion status when no column moves with it

A Draft/Published entity whose localized fields are all newly added has nothing
to copy from the main table, but it still needs a default-locale companion row
carrying the main row's status. Without one, every published row drops out of
locale-aware published reads. The seeding plan already handles an empty column
set for exactly that reason; the early return was preventing it from being asked.

Also states what the meta-service handle really requires: `BaseService` resolves
its dialect through capabilities as well as taking the Drizzle handle, so the
assertion there is load-bearing rather than cosmetic.

* fix(nextly): stop reporting a failed seed as a lost create race

The plan is no longer a single CREATE. When it also carries a seed, a failing
INSERT left a table this call had just made, and the catch read that table's
existence as proof another process won the race — suppressing the error and
returning as if all was well. Every later run then returned early because the
companion existed, so the content stayed uncopied and unreadable for good.

Only a failure before anything executed can be explained by a concurrent winner,
so the race check is limited to that case and everything after it is reported.

* fix(nextly): copy content into the companion before the apply drops it

Enabling localization removes the translatable columns from an entity's desired
main table, so the apply wants to DROP them. Provisioning ran only afterwards,
which left the copy either never happening — the drop was classified destructive
and the entity deferred — or happening too late, with the operator having
confirmed and the values already gone. The path meant to preserve content could
not reach it either way.

A pass before the apply copies first, so the DROP that follows is a cleanup
rather than a loss and it no longer matters which way the prompt is answered.

Restricted to entities whose main table already exists: those are the only ones
holding content worth copying, and a companion's foreign key needs a main table
the apply has not yet created for anything newer. Those stay with the pass after
the apply, which is also where column reconciliation belongs, since the apply is
what produces the columns it looks for.

* fix(nextly): copy content before db:sync pushes the schema too

`db:sync` had the same ordering as the dev reload path: the pushes run first and
companion provisioning followed, so an entity gaining localization could have its
translatable columns dropped before anything copied them. The reproduction passed
only because a non-interactive sync skips the destructive drop.

Both paths now share one existence helper rather than each asking their own way,
and both run the copy in a pass before the schema changes.

* fix(nextly): record the transition before the companion DDL runs

The record was written after `ensureCompanionTable` returned, which contradicts
the invariant the state module states for itself. MySQL commits DDL implicitly,
so a crash between creating the table and writing the record left a companion
whose next run sees the table, takes the early return, and never records or
completes the transition.

The same window made a failed copy unrecoverable. With the record already
written, a later pass can read `enabling` and finish what the interrupted run
started.

`ensureCompanionTable` now takes the write as a callback and invokes it once the
companion is known absent and before any statement runs, so the ordering holds
without each call site probing separately. A failure there abandons the creation
rather than proceeding without a record, leaving the next run a clean position to
retry from.

* fix(nextly): refuse to create a companion over content of unknown language

Boot-time provisioning has no localization config, so it passed no source locale
and created the companion empty. Creation is a race, and reads resolve through
the companion once it exists — so whichever caller won decided whether existing
content stayed visible, and a boot that beat `db:sync` hid it permanently, with
the seeding path then finding the table already there and skipping forever.

A caller that cannot say which language the main table's content is in now
declines to create the companion at all when the main table still carries the
translatable columns, and reports why. The entity is left to the path that knows,
and #382's write guard keeps a non-default write from doing damage until then.

Closing this by refusing rather than by teaching every caller to supply a locale
keeps it closed for callers that do not exist yet.

* fix(nextly): restore from the companion even when main kept its columns

Disabling localization re-adds the translatable columns to the main table and
copies the default locale back. That assumed main had none of them, which stopped
being true once unattended provisioning could seed a companion without dropping
the columns it copied from: `ADD COLUMN` is not idempotent on any supported
dialect, so the disable failed outright.

Skipping the restore for a column that is present would be worse than the error
it replaces. Presence says only that the column exists. Every localized write
since the transition went to the companion alone, so a retained column still
holds whatever it held before the entity was localized, and treating it as an
already-completed restore silently reverts an editor's work.

So the add is skipped for columns already there and the restore always runs.

* fix(nextly): tell the disable path which columns main still has

The guard added for the disable direction was inert: nothing populated
`existingMainColumns`, so a retained column still failed the re-add. The four
Builder dispatchers now supply it from the same physical lookup the create path
uses, so both directions read one fact rather than each introspecting to its own
shape.

* fix(nextly): keep writes working when a required column is retained

Three interactions between the fixes already on this branch.

A field that was required before localization leaves a NOT NULL column on main
when the seed keeps its source columns. The value then goes to the companion, so
the main insert omits it and the constraint fails every create. The column is now
relaxed as part of the transition — restated on MySQL, which has no direct form,
and dropped on SQLite, which cannot change nullability at all and whose only
alternative is rebuilding the table. Its value has just been copied, so dropping
loses nothing, while retaining it breaks writes outright.

The locale-less guard tested only for columns on main, but the seeding plan now
treats a Draft/Published entity as needing a seed even with no column to copy. A
boot caller could therefore create the very companion that plan would have filled,
leaving published rows out of locale-aware reads. It now defers on status too.

`db:sync --watch` still provisioned only after its pushes. Same defect as the
one-shot command, third call site.

* feat(nextly): finish a copy an earlier run started and abandoned

The transition record was written and never read, so the recovery it existed for
did not happen. `CREATE TABLE` and the copy are separate statements, and MySQL
commits DDL implicitly, so a failure between them leaves a real companion holding
none of the entity's content — after which every run returned early because the
table was there, and the content stayed hidden for good. Reporting that failure,
as this branch already did, told an operator about a state they could only leave
by editing `nextly_meta` by hand.

Provisioning now consults the record when the companion exists: still `enabling`
means the copy is owed, and this run finishes it. Only rows without a
default-locale companion row are copied, so a partial companion keeps what it has
and a complete one is a no-op — which is what makes the retry safe to repeat
rather than a second chance to collide on the primary key.

* fix(nextly): forget a transition once its companion is gone

The record is keyed by kind and slug, which a later entity can reuse, and it
outlived the thing it describes. Disabling localization drops the companion and
puts the values back on main, so the transition the record names no longer
exists — but it stayed, and if the default locale changed before localization was
enabled again, the check that protects a live transition refused the new one's
real source locale instead. The entity became impossible to re-localize without
editing `nextly_meta` by hand.

The three Builder disable paths now forget it once the drop succeeds, so the next
enable records what is true then. Deleting rather than settling is the point:
what the record protects is a transition in progress, and once there is no
companion there is nothing left to protect.

Entity deletion has the same problem and is not covered here. It reaches i18n
teardown through eight call sites that carry a slug but no entity kind, and the
kind is half the key.

* fix(nextly): only refuse creation when there is content to hide

The guard added for locale-less callers deferred whenever the entity had
Draft/Published, whether or not it held anything. That is every status-enabled
entity on a fresh database, so boot-time provisioning stopped creating their
companions and their localized writes were refused with "translations are not
ready" — eleven integration files across all three dialects.

What makes creating unsafe is content, not shape. A table that does not exist, or
exists with no rows, has nothing an empty companion could mask, and a new entity
has to be able to get its companion from any caller. Rows present, either
translatable columns on main or a per-row status is enough to defer, since both
are values the seeding plan would have carried across.

* test(nextly): run the seeding path against every dialect

The reproduction for this fix builds a SQLite adapter directly, so it ran on
SQLite even during the postgres and mysql legs — the copy that moves a user's
content had never executed against either server. The statements differ per
dialect in identifier quoting and in how a status column is carried, and a
unit-tested generator is not the same claim as a working copy.

Drives `ensureCompanionTable` rather than the CLI sequence, since the
orchestration is already covered and what was unproven is the seed reaching a
real server. Disabling the seed for mysql alone fails one case and leaves the
other eleven green, which is the discrimination the previous coverage lacked.

* test(nextly): cover the enable, edit, disable round trip

Every part of this was proven separately and the journey itself was not. An edit
made while localized lives only in the companion, so a disable that trusts the
retained main column instead of reading the companion reverts it — silently, to
whatever the value was before localization was turned on.

Skipping the restore for a column already present, which is what the disable path
did before this branch, fails it on all three dialects with the pre-localization
value where the edit should be. That is the failure a user would have reported as
losing their work.

* fix(nextly): forget a transition when its entity is deleted

The record lives in `nextly_meta`, so nothing the teardown dropped removed it,
and it is keyed by kind and slug — both of which a later entity can reuse. A slug
recreated and localized under a different default hit the refusal that protects a
live transition, after its companion had already been created and seeded, and the
entity could not be localized without editing `nextly_meta` by hand.

The kind was the obstacle: teardown carried a slug and nothing else. Seven of its
eight callers know their kind statically, and the eighth is the catalog sweep that
already passes a null slug because it cannot identify the entity at all. Making
identity a union rather than two optional fields let the compiler name every site
that had to supply one, and leaves the guessing shape unwritable.

Guarded on the meta table existing, like the archive purge above it. A database
that never finished core setup still has to be able to delete an entity, and
failing over a bookkeeping row would block the drop this exists to perform —
which is what it did before the guard, on the first run against a real database.

* fix(nextly): settle the transition and resume it in its own language

Two faults in the record's lifecycle, both of which made the marker describe
something other than what happened.

Nothing ever settled it. `settleI18nTransition` had no production caller, so a
completed copy stayed recorded as owed and every later sync or reload re-ran it —
harmless for the rows already there, but it kept manufacturing default-locale
rows for entries deliberately created in another locale only. Both the initial
copy and a resumed one now settle, and only once every statement has landed.

The resume also used the locale configured now rather than the one recorded. A
default locale changed since the interrupted run would have relabelled the old
main-table values with the new language — the exact substitution the record
exists to prevent. `seedIncomplete` answers with the recorded locale instead of
a bare yes or no, so there is nothing left for the caller to supply.

The reproduction now restores the marker to `enabling` as well as emptying the
companion, because a settled transition is legitimately not resumed and the old
setup no longer described an interrupted run.
mobeenabdullah added a commit that referenced this pull request Jul 31, 2026
…aborting transactions (#429)

* fix(nextly): restore content when localization is turned off in config

Provisioning skipped every entity that was not currently localized, so setting
`localized: false` in configuration abandoned the companion holding all the
content and fell back to the main table's retained columns. Those hold whatever
they held before the entity was localized, because every write since had gone to
the companion alone, so the edits were still on disk and no longer visible.

The restore runs after the schema push, which is what puts the columns back, and
copies the default locale's companion values onto main. It drops and archives
nothing: db:sync persists registry metadata before its destructive prompt, so a
drop would run even for an operator who then declined the change, and with the
companion still standing there is nothing an archive would preserve.

Leaving the companion in place would otherwise create the mirror trap, so the
transition record gains a `restored` state. Re-enabling localization then
overwrites those default-locale rows from main instead of trusting them, since
main was authoritative for the whole period localization was off.

The store is now resolved without requiring a configured default locale. An app
that removes its `localization` block entirely is exactly the case that owes a
restore, and demanding a locale first hid those entities; the recorded source
locale stands in when configuration no longer names one.

* fix(nextly): seed status from the physical column and claim the marker atomically

Two defects that both come of trusting something other than what is actually
there at the moment the statement runs.

The seed read `status` from the main table whenever the entity had
Draft/Published in its desired configuration. One edit turning on localization
and Draft/Published together reaches the pre-apply copy with no `status` column
yet, so the seed failed after the companion had been created — and every later
run found that companion and resumed into the same statement, so that
combination could never apply. The status copy is now gated on the physical
column. Nothing is lost by skipping it: main's column and the companion's
`_status` are both created NOT NULL DEFAULT 'draft', so rows gaining
Draft/Published in this edit reach the same state either way.

Recording the first transition went through a check-then-act write, so two
processes provisioning the same entity both read `untracked` and both wrote. The
one that lost the companion CREATE could still record the language, leaving the
marker naming a locale the seed never used — which defeats the only fact the
record exists to keep. The first write is now a conditional insert resolved by
the database, and the caller re-reads to see what landed: agreeing on the locale
is not a loss worth reporting, disagreeing is fatal.

* fix(nextly): keep local schema out of generated migrations and refresh the pre-apply snapshot

Three findings, all about a decision outliving the state it was made from.

A disable migration took its prior main-table shape from live introspection, so
a file generated against a development database whose unattended transition
retained its columns omitted their ADD COLUMN. Replayed where the enable
migration had dropped them, the restore addressed columns that were not there.
The artefact is now derived from history alone and the local shape produces a
second plan, returned only when it differs, so a caller holding one plan cannot
pick the wrong one. The file is what is saved; the local plan is what is run.

The pre-apply transition can relax a retained column, and on SQLite — which
cannot change nullability — drops one instead. The reload had already cached the
live snapshot the pipeline reuses, so the apply re-emitted a DROP for a column
that was already gone. The cache is dropped when the pass changed anything,
which costs one introspection and only in a cycle where a transition happened.

A comment named a pull request. Comments describe code.

* refactor(nextly): resolve companion readiness once instead of probing every write

Three consumers were asking the database the same question — provisioning, the
localized write guard, and the read-back that builds a response — and each asked
it its own way, per write. An entry whose dynamic zone holds K localized
field-group types paid 1 + K round trips before its transaction and K more
inside it. Local SQLite hides that; managed PostgreSQL does not, and the cost
grows with the complexity of the content.

Readiness is now three states rather than a boolean, because "no companion"
splits into a legitimate main-table fallback and a state that must be refused,
and a boolean would leave the introspection that tells them apart on the write
path. Only `ready` is remembered: it is the healthy steady state where the whole
per-write cost lives, and it is reached by creating a table, which no ordinary
operation undoes. The abnormal states re-resolve every time, so an entity
mid-transition keeps the freshness it has today rather than trading it for
speed it does not need.

That also closes the read-path abort. companion-join decided the companion
existed by running the join and catching the failure, at five sites — free on
SQLite and MySQL, and on PostgreSQL a statement that errors marks the whole
transaction aborted, so the next one dies with `current transaction is aborted`
and takes the blame. Several of those reads run inside the caller's write
transaction, so the check was causing the failure it was written to tolerate.
Readiness is now an argument, required on every reader so a new caller cannot
omit it, and nothing in that module catches: not ready means no query, ready
means every failure propagates. The `strict` flag existed only to opt out of the
swallow and goes with it.

The presence map #382 threaded from a pre-transaction pass through three
services collapses into the same lookup, since resolving readiness before the
transaction is what the in-transaction path now reads.

086's reproduction, removed from #382 because it could not pass, is restored. It
runs against a suite that can finally see the failure: the merged
aborted-transaction guard fails any run that leaves a transaction poisoned, so a
reintroduction trips on its own.

* feat(nextly): give localization a supervised repair path and an honest refusal

The refusal an editor hits when translations have nowhere to go named
`nextly db:sync` in every environment. In production that is wrong advice: boot
deliberately refuses to run DDL there, so the development remedy cannot help,
and naming it costs the operator the time to try it before they start looking
for the real answer. The message now branches, and the three copies of it that
had begun to drift are one function.

`nextly migrate` becomes the remedy it names. It provisions companions after the
migrations have run, which is the only supervised path available to a deployment
that may not alter its own schema at boot, and the only one that can repair an
install which transitioned before transitions were recorded. Those have a
companion and no marker, so nothing can tell whether their content was ever
copied across — the one fact that cannot be re-derived is the language, and
running the repair supplies it from the configured default. The copy is guarded
on rows with no default-locale companion row, so an install that does not need it
gets a scan and nothing else, and translations written since are never
overwritten.

Absence is read as a debt ONLY under supervision. An entity localized from birth
is untracked too and owes nothing, so an unattended pass must keep treating the
two the same.

* chore(release): add changeset for the i18n companion readiness work

* fix(nextly): claim a transition before a new copy and normalize companion read failures

Three review findings. Two are one missing invariant.

A seed debt that continues an existing `enabling` record can go straight to the
copy — that record is what it continues. The other two ways a debt arises are
NEW transitions and were not recording one, which broke both of them.

The supervised repair copied and then called settle, and settle refuses an
entity with no record: nothing established that a copy ran or in which language.
So `nextly migrate` reported a provisioning failure after modifying the
database, left the marker untracked, and failed identically on every retry.

A companion that outlived a disable reused the locale the restore recorded. Main
has been authoritative since that disable and carries no language of its own, so
enabling now declares its content to be in TODAY's default, exactly as a first
enable does. Reusing the restore's locale labels the rows with a code reads no
longer look for — one that may not even be configured any more — so every edit
made while localization was off disappears the moment it comes back on.

Both now claim a transition at the current default before any copy, which is
also this module's own ordering rule: the record goes in before the statements,
because MySQL commits DDL implicitly.

Separately, a Single read no longer hands the driver's own words to the
response. Companion reads used to swallow a failure, so only the access-rule
path could throw and only that path wrapped the error; now every failure
propagates, and the result builder was putting the failed query — companion
table and column names included — straight onto the wire.

* fix(nextly): scope readiness per connection and make claims and restores atomic

Four review findings, each a place where something that looked settled was not.

Readiness was remembered by table name alone, on a process-wide set. A table
name does not identify a table: one process can hold two adapters, and the first
database's verdict then vouched for a companion the second had never
provisioned, so its reads and writes addressed a missing table instead of taking
the pre-migration fallback. Keyed on the adapter now, through a WeakMap, so
identity does the scoping and a discarded adapter takes its verdicts with it.
The clearest evidence it was mis-keyed is what the fix deletes: the package test
setup no longer has to wipe the cache after every test.

Re-enabling a `restored` entity took an unconditional write. Two processes doing
that during a default-locale rollout both read `restored` and both proceeded
under their own locale, labelling one main table's content as two languages
while the marker recorded whichever landed last. It is a conditional move now,
resolved in the database, with the loser re-reading and refusing when the winner
chose differently. `MetaService` gains the compare-and-set that pairs with the
insert-if-absent added earlier: one settles a race to create a key, the other a
race to move one.

A multi-column restore ran one statement per column, so a failure part-way left
main carrying a mixture of restored and pre-localization values with nothing
recording that a restore was attempted — after which the app served that
mixture, accepted edits on it, and the next pass overwrote them from the stale
companion. One statement covers every column, so it either happened or it did
not.

`nextly migrate --step` provisioned companions from the final config while later
migrations were still pending, creating a companion that a pending migration was
about to create for itself. Provisioning now waits until nothing is pending, and
says so.

* fix(nextly): stop inferring a localization repair, and refresh status on re-enable

Four review findings.

The supervised repair read the absence of a transition record as a debt. It is
not: an entity with no record is either an install that enabled localization
before Nextly recorded transitions, or one localized since birth that owes
nothing — and nothing on disk tells them apart, since the push leaves the
translatable columns on main in both cases. Inferring it manufactured a
default-locale translation for every entry, including ones deliberately authored
in another language only, and recorded a transition that never happened. It is
now `nextly migrate --repair-localization`, which is the operator supplying the
one fact that cannot be recovered: that their main tables hold content in the
configured default locale. That is the same conclusion the rest of this
mechanism rests on, applied to the one place I had reached for a guess.

The transition is also no longer recorded while resolving the debt, only once
the plan is known to describe real work, so a repair that turns out to be owed
nothing leaves no trace.

Re-enabling refreshed the localized values and left `_status` alone. Publishing
state moves while localization is off too, and the companion row that survives
the disable keeps whatever status it had when it was last the authority — so a
page published in the meantime stayed hidden from locale-aware published reads,
and the guarded insert could not correct it, because the row it would fix
already exists.

Collection reads now normalize a companion failure the way the Single reads do.
These reads used to swallow one, so nothing here had to shape their errors; now
every failure propagates and `listEntries` was putting the failed query — with
companion table and column names in it — into its own result message.

A failed restore during an HMR reload only warned, and the reload went on to
publish the non-localized configuration. The app then read the stale main values
and accepted edits on them, and a later successful retry copied the companion's
older values over the top. It now stops, exactly as the pre-apply preservation
path does, and says the content is intact where it is.

* refactor(nextly): run the runtime localization restore through Drizzle

The restore executed generated statement strings through `adapter.executeQuery`,
which is a raw-SQL product data-access path and against the repository's
Drizzle-only rule.

It now goes through the query builder. Both table objects already have runtime
builders — `generateRuntimeSchema` for main, `buildCompanionRuntimeTable` for the
companion — so identifiers come from the generated columns rather than
hand-quoting, and the locale is bound rather than embedded. The correlated
subquery uses Drizzle's `sql` template with those column references, which is
how a correlated copy is expressed in the builder.

The pairing is the part worth knowing: the main table object is keyed by FIELD
name while the companion is keyed by physical COLUMN name, so `subTitle` and
`sub_title` are one value under two keys. They are paired through the same
descriptor the columns were created from rather than by re-deriving the
conversion.

`buildDefaultLocaleRestoreStatements` stays, and stays tested: the disable
MIGRATION still emits text, because a migration file has to carry SQL. What goes
away is a runtime path executing that text.

* fix(nextly): make the restore pick a locale that exists and stop unconditional marker writes

A review pass over the whole branch. The three that could lose or strand content:

The restore preferred the configured default locale unconditionally. Its copy is
guarded on a matching companion row, so an entity only ever authored under the
locale the transition recorded matched nothing, copied nothing — and the record
still marked the transition finished. `restored` is terminal, so no later pass
retried and the content stayed in a companion nothing reads. The locale is now
chosen by which one the companion actually holds.

A field turned `localized: false` while its entity stays localized keeps its
companion column, because reconciliation is additive, while writes correctly go
to the restored main column. A later entity-level disable then copied that
abandoned value back over the current one. The per-field flags now decide which
columns the companion still owns, and only fall back to the physical
intersection when none of them claim anything.

`settleI18nTransition` and `recordI18nRestore` wrote unconditionally. Opposite
transitions can interleave — a disable can restore and record while a re-enable
is still copying — and an unconditional write buries the other's state, after
which the next enable trusts a companion that is stale. Both are conditional on
the state they actually operated on now, and losing is not an error: whatever
moved the entity owns it.

Also: `confirmClaim` compared only the locale, so a claim lost for any
non-concurrency reason passed as won and left the copy with nothing to settle;
it now requires `enabling` too. A NULL main `status` violated the companion's
NOT NULL `_status` on refresh, permanently, since the transition stayed
unsettled and every pass replayed it. The companion upsert on the singles write
path was not gated on the companion existing, so a payload carrying a status
rolled back the very fallback write it was meant to allow. `insertIfAbsent` fell
back to a plain insert when neither conflict clause was found, silently turning a
claim into an unconditional write. The no-DDL reload path discarded
`restoreFailed`, so a disable that produced no schema diff published the
non-localized metadata anyway. Companion absence is confirmed through the write
path's own probe before the record carrying the source locale is deleted. The
disable planner's `existingMainColumns` is filtered to fields that were actually
translatable. And two builder headers still described one statement per column
after both became one statement covering every column.

* fix(nextly): warm readiness before transactions and bound a positive verdict

Five findings, four of them the same shape from different directions: a verdict
that is only ever READ inside a transaction has to be RESOLVED before one opens,
and a verdict that outlives the schema it describes is worse than none.

Deleting a localized entry on a fresh worker resolved nothing beforehand, so the
in-transaction snapshot that builds the durable delete event read an unresolved
verdict, treated it as unusable, and silently omitted every localized field.
Readiness is warmed on the pool before that transaction opens.

The same for field groups: the pre-transaction pass resolved only the component
types the payload happened to write, while a version or webhook snapshot reads
every component the entity holds. A type left out of the payload had no verdict
inside the transaction and its localized values went missing from the durable
record. Every PERMITTED type is now resolved. Resolving is not refusing — the
refusal still walks only what the payload writes, because a permitted type whose
companion is missing must not fail a save that never mentions it.

A positive verdict was trusted for the lifetime of the adapter. Companions are
dropped by disable migrations, and `nextly migrate` runs in a process that
cannot reach into a live server's memory, so an old worker in a rolling
deployment kept querying a table the database no longer had. There is no
invalidation channel between those processes, so the staleness is bounded by
time: thirty seconds, which costs one plan-only SELECT per entity per window on
the paths that resolve — against one per write before any of this.

The re-enable refresh still executed a generated statement string. It goes
through the query builder now, like the restore, and both directions live in one
module so the pair cannot drift and the fact they share — that the main table
object is keyed by FIELD name while the companion is keyed by physical COLUMN
name — is written down once.

* fix(nextly): restore each entry from the locale it has, and defer DDL-backed disables

Two findings, both consequences of the previous round.

The no-DDL provisioning pass ran before the `hasChanges` check, so a disable that
needs DDL to put the main columns back reached it before the apply had added
them. The restore found nothing to copy, copied nothing, and still recorded the
transition as finished — after which the post-apply pass skipped it and the
recreated columns stayed empty while the content sat in a companion nothing
reads. It runs inside the branch now, which is the only place it was ever for.

The restore also chose one locale for a whole entity. The configured default can
move while an entity is localized, so a corpus ends up mixed: some entries
authored under the new code, some only under the one the transition recorded.
One choice restores whichever group matches and leaves the rest holding
pre-localization values, with the record marking the transition terminally
finished so nothing retries. Each entry is now restored from its preferred
locale when it has a row there and from the recorded one otherwise, still in a
single statement, still guarded so an entry with a row in neither is left alone
rather than blanked.

* fix(nextly): warm field-group readiness before the payload is consulted

The warming loop sat after the check that skips a field the payload does not
mention, so the commonest way to reach the problem — omitting a component field
entirely — never reached it. A snapshot reads every component the entity holds,
not the ones a save happens to mention, and it reads them inside the caller's
transaction where a verdict can only be read. Warming now runs for every
field-group field before the payload is looked at.

`publishAllLocales` built the same snapshot without warming anything at all. It
resolves the collection's readiness and every field-group type it can hold
before opening its transaction; nothing is written, so nothing is judged, and the
call is there purely for the verdicts it leaves behind.

* fix(nextly): settle only from the state the copy was claimed under

Both completion writes derived their expected value from a re-read taken AFTER
the copy they complete, so a transition that landed in between was accepted as
the thing being completed rather than as the reason to stand down.

Settling is now only ever from `enabling`, which is the only state a settlement
can truthfully follow: a disable that restores the content and records
`restored` while the copy is still finishing would otherwise be moved to
`seeded`, telling the next enable that the companion is authoritative and
reverting every edit made on main while localization was off.

The restore's completion takes the state observed before its copy as the
expected value, so a re-enable that claimed the entity mid-copy keeps its claim
instead of having it overwritten by a completion it never saw.

* fix(nextly): hold a transition exclusively and restore its publishing state

A claim needs a token, not agreement. Two callers reading one configuration
necessarily name the same source locale, so a loser that checked only the locale
found its own answer looking back and took it as permission to do the work
again. For a re-enable that work is a destructive refresh, whose second pass
lands after the winner settles and copies stale main-table values over
translations written since. Every claim now stakes a unique token and only its
holder proceeds; a marker written without one stays claimable, so nothing in
flight is stranded.

The disable restore consulted the per-field flag literally, but sharing has two
spellings and only one is written down: a text field with no flag localizes by
its type default. With every remaining field defaulted and one field explicitly
shared, nothing read as claimed, the fallback offered every field, and the
abandoned companion value was copied over the shared field's current one. The
pipeline's own classifier decides now. Clearing all the flags at once still
falls back to the physical intersection, because that says nothing about what
the companion held while it was on.

That restore also moved values without the state they were published under.
Publishing is per locale while an entity is localized, so an entry published
only under a non-default locale carries it on its companion row alone; bringing
the content back without it either makes a draft public or makes live content
vanish. The status travels with the values now, from the same row, gated on both
tables physically carrying the column and read from the snapshot that already
resolved the columns.

Batch writes resolve companion readiness before they open their transaction.
Inside one it can only be read, and an unresolved verdict reads as unusable, so
a worker whose first act was a batch committed its rows while their version
snapshots and outbound events silently lost every translated value. The same
resolution is exposed publicly for callers that own the transaction themselves,
because nothing throws when it is skipped and the loss surfaces from a consumer
long after the snapshot became the historical record.

* docs(release): note the restored publishing state and the readiness warm-up

The changeset described what this PR did before its last three fixes. Restoring
publishing state with content, refusing a second holder of one transition, and
the warm-up a caller-owned transaction needs are all things a user acts on.

* fix(nextly): restore each entry from one companion row, and settle only your own claim

Restoring took each column's first non-null value across the candidate locales
independently. A parent holding rows in BOTH, with one field untranslated in the
preferred one, therefore took that field from the other language while its
neighbours and its publishing status came from the preferred row: a
mixed-language document written to the table that is authoritative from then on,
with the record marking the restore terminally finished. The row is chosen once
per parent by rank now, and every value comes from it.

An entity with no transition record was skipped entirely, which stranded exactly
the installs that predate those records. Whether a companion came from a legacy
transition or from an entity localized since birth makes no difference when
localization is being turned off: either way the content is in the companion,
because every write since went there. What separates both from an entity that
was never localized is that the never-localized one has no companion at all, so
that is what decides now. The transition is established through the ordinary
claim first, so two processes disabling at once cannot both copy, and one cheap
probe answers before any introspection because almost every entity reaching here
never had a companion.

Taking over an unfinished transition is how a crashed run gets recovered, and
nothing in the row distinguishes an abandoned claim from an active one — a
wall-clock lease cannot either, since a copy over a large table outlasts any
timeout while its holder is still running. So the takeover is made harmless
rather than forbidden: a claim is settled only by the token that made it, and a
holder displaced mid-copy can no longer declare the copy done on behalf of the
claim that displaced it. Resuming an unfinished copy claims too, which both lets
it record that it finished and serialises two runs resuming at once.

The reload fixture's adapter resolved every raw statement, so the companion
existence probe reported a companion for every entity and certified paths that
cannot run against a database without one. It answers with a missing-table error
now unless the fixture says otherwise.

* fix(nextly): rank restore locales instead of filtering by them

Naming the locales a restore may copy from turned them into a filter, and the
case where that bites hardest is the one where the names are weakest: removing
the localization block leaves no configured default at all, so the only
candidate is the locale recorded when localization was first switched on. An
entry authored solely under a default adopted since then has no row there, keeps
whatever main held before it was ever localized, and is marked restored anyway.
The named locales rank the rows now instead of selecting them, with the locale
itself breaking the tie, so every parent comes back from the row it actually has
and only a parent with no row at all is left alone.

A field the configuration declares shared is excluded in every branch, not only
when something else is claimed. One made shared while its entity stayed
localized keeps a companion column that the physical intersection accepts, so an
edit that clears the last remaining flag in the same pass as the entity's would
otherwise hand back every field and copy that abandoned translation over the
value main has been authoritative for.

A component's translations failing to read no longer takes its shared values
with them. The overlay runs after the shared values are deserialized, and the
component read above it replaces the whole field with null when anything throws,
so a fault that costs the reader one translation was costing them the record.
Contained at the overlay, and only off a transaction: on the caller's connection
the failure has already aborted it, and there the error belongs to the query
that caused it rather than to whatever runs next.

* fix(nextly): carry the claim into the one statement that cannot be undone

Settling only your own claim stops a displaced holder from closing someone
else's, but it does not stop the work. A run that claims a re-enable, pauses,
and resumes after another run has taken over, refreshed and settled still
executes its refresh — overwriting from stale main-table values the translations
the taker has already seeded and published. Checking ownership beforehand cannot
close that, because the check and the update are separate round trips. So the
claim travels into the statement: the refresh carries a WHERE the database
evaluates alongside it, and a claim the row no longer names matches nothing.

Recording a restore reported nothing when its conditional write lost. The copy
has already written main by then, so a caller told nothing goes on to publish a
non-localized configuration over a record that says otherwise, and the next
enable trusts a companion that no longer describes the main table. The result is
returned and the pass fails rather than reporting a restore that is half true.

Deleting an entry warmed only the collection's own companion. The snapshot that
becomes the durable delete event reads every embedded component through the
transaction, where a verdict can only be consulted, so on a fresh worker those
overlays were skipped and the last description of the row there will ever be
went out without its translations.

* fix(nextly): rank locales in the disable migration and report a lost settlement

The migration that disables localization restored only the default locale, then
archived the other languages and dropped the companion. A parent with no
default-locale row was skipped by the guard and kept whatever main held before it
was ever localized, while its actual content left with the table. The default is
a preference now, as it already is at runtime: rows are ranked per parent and one
is chosen, so every entry comes back from the row it has. The guard moves to the
parent, which still leaves a row that never had a translation alone.

Settling a seed reported nothing when its conditional write lost. A run whose
claim was taken over mid-copy therefore reported success, no preservation failure
was recorded, and the schema apply that follows was free to drop the
main-table columns — whose values the new claimant may not have copied anywhere
yet. The result is returned and provisioning abandons the entity instead.

Settling twice with the same token still succeeds. The marker keeps its owner
across the move, so a settlement recognises its own finished work rather than
reading it as a takeover.

* fix(nextly): restore publishing state in the disable migration too

Found by sweeping for fixes that landed on one side of a symmetric pair, after
three review rounds in a row caught exactly that mistake.

The runtime restore learned to carry a row's publishing state back with its
values; the disable MIGRATION never did. Publishing is per locale while an entity
is localized, so a row published only under a non-default language holds that
state on its companion row alone — and this path drops the companion immediately
after restoring, so moving the content without the state it was published under
puts a draft in front of the public, or takes live content down, with nothing
left to correct it from.

Gated on the entity having Draft/Published, because that is what puts `status` on
main and `_status` on the companion; reading either without it fails the whole
migration.

* fix(nextly): decide the disable status restore from the physical tables

A regression from the commit before this one. The status restore was gated on
`spec.status`, which is the shape the collection is being saved AS, not what its
tables currently carry. A save that disables localization and turns
Draft/Published on in the same edit therefore emitted a read of a `_status` the
old companion never had, into a `status` the main table has not been given yet —
a disable deliberately runs the companion transition before the shared ALTER that
adds it. The migration fails after it may already have re-added the localized
columns, while the registry has recorded the entity as no longer localized.

The verdict now comes from the caller, which knows both physical shapes: the
existing companion's `_status` and the main table's `status`. Omitted means leave
status alone.

This is the same mistake the runtime restore already carries a comment about —
that the desired schema cannot answer whether the columns are there — reproduced
in the generator while fixing an unrelated gap in it.

* fix(nextly): remember what a claim owes, and guard the copy that cannot be undone

The fact that a re-enable must OVERWRITE a surviving companion lives in the state
it claims from — `restored` — and claiming replaces that state with `enabling`. A
run that crashed in between left a marker indistinguishable from an ordinary
unfinished seed, so the retry did the guarded insert, skipped the stale rows,
settled, and left them hiding every edit made while localization was off. The
claim records what it owes now, a takeover carries it rather than downgrading it,
and settling clears it because the work is done.

The restore copy carries its transition into the statement, as the re-enable
refresh already did. Two processes disabling one entity can both read the same
marker and reach it; if the first finishes and publishes the non-localized
configuration, edits land on main and the second overwrites them from a companion
that is stale by then. Noticing the lost record afterwards cannot bring those
edits back.

Restoring an untracked companion no longer needs a configured default. Removing
the localization block outright is when an entity has neither a record nor a
default — and the copy stopped needing a locale named once it began ranking each
parent's own rows, so all that was missing was something true to write in the
record. A locale the companion demonstrably holds is that.

The all-locale component overlay returned before the containment the single-locale
branch had, so a drifted companion still cost the reader the whole component
rather than its translations. Both now run through one helper.

A metadata update that drops a companion forgets its readiness, as the dispatcher
disable paths already do. And losing the CREATE race after claiming the transition
is reported rather than read as a quiet non-creation: this run holds the marker
for a table another made, that run may have died before seeding it, and a caller
told nothing lets the apply drop the columns whose values never got across.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: core nextly type: docs Documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant